Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the editor file import UI to support markdown tiddlers #8486

Merged
merged 7 commits into from
Oct 2, 2024

Conversation

saqimtiaz
Copy link
Member

@saqimtiaz saqimtiaz commented Aug 5, 2024

This PR updates the import UI in the editor that is triggered by drag and drop to be reusable in x-markdown tiddlers.

  • All macros have been updated to procedures, which is a safe change as none of the macros used text-substitution
  • the UX tiddler has been updated to reuse the filter from $:/config/Editor/EnableImportFilter which defines whether importing in the editor is supported for that tiddler type

This PR explores an alternative to #8463 to reduce code duplication and closes #8388

Copy link

github-actions bot commented Aug 5, 2024

Confirmed: saqimtiaz has already signed the Contributor License Agreement (see contributing.md)

@Leilei332
Copy link
Contributor

Leilei332 commented Aug 8, 2024

In my opinion, one tiddler shouldn't handle more than one types of links. This is because of these reasons:

Different types of links needs to be handled differently. For example, markdown internal links requires escaping characters like ()<>\ while wikitext internal links does not. IMO mixing them to let it handle two types of links can make the code become complicated.

I think a better solution is to abandon the $:/config/Editor/EnableImportFilter so that the file import UI can be used for any type when there is one available (since those without a handler didn't have side effects at present), like the editor toolbar icons.

For the code duplication problem,since the core logic is the same, I think we can create a template and require the handler to provide essential information (like the link template, characters to escape) to solve it.

@saqimtiaz
Copy link
Member Author

I think a better solution is to abandon the $:/config/Editor/EnableImportFilter so that the file import UI can be used for any type when there is one available (since those without a handler didn't have side effects at present), like the editor toolbar icons.

This isn't a possibility due to backwards compatibility constraints and the fact that showing an affordance to import something and having nothing happen when you drop a file would be very misleading and would be construed as a bug. Also, it is a useful feature to be able to turn off the ability to import into the editor as well.

For the code duplication problem, since the core logic is the same, I think we can create a template and require the handler to provide essential information (like the link template, characters to escape) to solve it.

I have considered a similar solution where we lookup the template to use in a given tiddler type and if it does not exist, fall back to wikitext. This would allow plugins to provide the templates to use for different tiddler types. However, given that there is unlikely to be another plain text format that needs to be supported, this does feel like an unnecessary amount of complexity.

markdown internal links requires escaping characters like ()<>\ while wikitext internal links does not.

Escaping characters for markdown can be easily incorporated into this PR.

@saqimtiaz
Copy link
Member Author

@Leilei332 I have updated the code to URI encode tiddler titles for markdown tiddlers, which the Markdown plugin documentation states is an alternative to escaping characters:

If a tiddler title contains spaces or other special characters, you must either (1) URI-encode the title, or (2) surround the #title with < > and backslash-escape any < or > in the title.

@Leilei332
Copy link
Contributor

@Leilei332 I have updated the code to URI encode tiddler titles for markdown tiddlers, which the Markdown plugin documentation states is an alternative to escaping characters:

If a tiddler title contains spaces or other special characters, you must either (1) URI-encode the title, or (2) surround the #title with < > and backslash-escape any < or > in the title.

In my opinion, I preferred escaped characters to URI encoded characters, since they don't make non-ASCII characters unreadable (as is described in #8402).

@saqimtiaz
Copy link
Member Author

Updated to use character escaping instead of URI encoding of tiddler titles used in image syntax in markdown tiddlers.

@Jermolene
Copy link
Member

Hi @saqimtiaz is this ready to come out of draft as suggested by @Leilei332 over in #8525 (comment)?

@saqimtiaz saqimtiaz marked this pull request as ready for review October 1, 2024 16:27
@saqimtiaz
Copy link
Member Author

@Jermolene this should be good to go. I prefer this implementation over the one in #8525 as explained here.

@Jermolene
Copy link
Member

Thanks @saqimtiaz

@Jermolene Jermolene merged commit e591dfd into TiddlyWiki:master Oct 2, 2024
3 checks passed
@saqimtiaz saqimtiaz deleted the image-import-markdown branch October 3, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IDEA] Support image drag & drop (cut & paste) for markdown tiddlers
4 participants